Skip to content

Conversation

aliemjay
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

celinval and others added 7 commits December 13, 2023 16:36
Late bound regions were still part of the signature.
- Remove `fn_sig()` from Instance.
- Change return value of `AssertMessage::description` to `Cow<>`.
- Add assert to instance `ty()`.
- Generalize uint / int type creation.
…iler-errors

Erase late bound regions from `Instance::fn_sig()` and add a few more details to StableMIR APIs

The Instance `fn_sig()` still included a late bound regions which needed a new compiler function in order to be erased. I've also bundled the following small fixes in this PR, let me know if you want me to isolate any of them.

  - Add missing `CoroutineKind::AsyncGen`.
  - Add optional spread argument to function body which is needed to properly analyze compiler shims.
  - Add a utility method to iterate over all locals together with their declaration.
  - Add a method to get the description of `AssertMessage`*.

* For the last one, we could consider eventually calling the internal `AssertKind::description()` to avoid code duplication. However, we still don't have ways to convert `AssertMessage`, `Operand`, `Place` and others, in order to use that. The other downside of using the internal method is that it will panic for some of the variants.

r ? `@ouz-a`
Opportunistically resolve region var in canonicalizer (instead of resolving root var)

See comment in `compiler/rustc_type_ir/src/infcx.rs`.

The **root** infer region for a given region vid may not actually be nameable from the universe of the original vid. That means that the assertion in the canonicalizer was too strict, since the `EagerResolver` that we use before canonicalizing is doing only as much resolving as it can.

This replaces `resolve_lt_var` and `probe_lt_var` in the `rustc_type_ir` API with `opportunistic_resolve_lt_var`, which acts as you expect it should. I left a FIXME that complains about the inconsistency.

This test is really gnarly, but I have no idea how to minimize it, since it seems to kind of just be coincidental that it triggered this issue. I hope the underlying root cause is easy enough to understand, though.

r? `@lcnr` or `@aliemjay`

Fixes rust-lang#118950
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Dec 15, 2023
@aliemjay
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Collaborator

bors commented Dec 15, 2023

📌 Commit 9a07292 has been approved by aliemjay

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2023
@bors
Copy link
Collaborator

bors commented Dec 15, 2023

⌛ Testing commit 9a07292 with merge d253bf6...

@bors
Copy link
Collaborator

bors commented Dec 15, 2023

☀️ Test successful - checks-actions
Approved by: aliemjay
Pushing d253bf6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 15, 2023
@bors bors merged commit d253bf6 into rust-lang:master Dec 15, 2023
@rustbot rustbot added this to the 1.76.0 milestone Dec 15, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#118927 Erase late bound regions from Instance::fn_sig() and add … ddf3e746d0d163f1d3263cf68bd24762e443e5df (link)
#118964 Opportunistically resolve region var in canonicalizer (inst… 3bc3445bc0d5c3b3bebc29153268016480c177d5 (link)

previous master: cca2bda07e

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d253bf6): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.3% [0.5%, 5.4%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) 2.3% [0.5%, 5.4%] 3

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.4%, 0.9%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.3% [-1.3%, -1.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.0% [-1.3%, 0.9%] 3

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.696s -> 672.135s (-0.08%)
Artifact size: 312.36 MiB -> 312.47 MiB (0.04%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants